fix(typing): drain 6 more modules (exempt list 26 -> 20, 79 of 99 checked) - #98
Merged
Conversation
…ecked
Batch three: 372 -> 338 findings, 25 -> 19 dirty modules (exempt bound 26 -> 20).
TWO SHADOWED-NAME BUGS the checker found, both the same shape as the `consumer_sync_shadow` one in
batch 1 and worth naming because it keeps recurring in this codebase:
* `repo_knowledge.main()` bound `repo` THREE times — a `str` for `--export-agents-md` and two
optionals for `--repo`. The optionals are renamed `repo_arg` along with every use inside their
blocks, so the two meanings stop sharing a name. `no-redef` was the only thing pointing at it.
* `capability_activation_audit`'s selftest reused `broken` for both a str and the note's return.
PATTERNS, fixed once at the declaration rather than at each use:
* `SEED` in `repo_knowledge` is a nested literal, so all five reads widened to `object`.
Annotating the constant once cleared them.
* accumulator dicts — `summary` in `durability_sweep`, `out` in `capability_activation_audit`,
`report_repos` in `consumer_sync_artifact_ingest` — whose literal pins a narrow value type and
then conflicts on `+= 1` / `.append(...)` / a wider entry.
* `cast`, not `list(...)`, for object-typed reads. `list(x)` does not satisfy mypy for an `object`
and adds a real copy; batch 2 taught this and two more sites had it.
THE `Mapping` FIX PROPAGATED, which is the right direction. Batch 2 changed `tick`'s `env` parameter
to `Mapping[str, str]`, and that pushed five errors DOWNSTREAM into callees still declaring `dict`.
`adversarial.review_enabled`, `adversarial.reviewers_from_env` and `roles`' env parameters now accept
a `Mapping` too — they only ever read from it, so `dict` was never what they needed.
Two narrow ignores, each with its reason: a deliberately mixed-type loop probe in
`durability_sweep`, and one `setdefault(...)` index in `capability_activation_audit` where the row is
genuinely heterogeneous. No error code is disabled anywhere.
`mypy_exempt_max` lowered 26 -> 20.
Verified: 442 passed, 0 failed, 0 skipped, 85/85 selftests, 43/43 can-fire, 5/5 gates. `ruff check .`
and `black --check --line-length 100` clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 55 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 73 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
Comment |
Contributor
Workflow source neededPR #98 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely. Please do one of:
Once a valid source is present, this warning will not be reposted. |
stranske
added a commit
that referenced
this pull request
Aug 24, 2026
…OMMAND (#100) `local_verify.verify()` ran `test_cmd` twice -- once in the worktree, once against the extracted base -- and returned ONE verdict from `red["ok"]`. `red["ok"]` goes False as soon as ANY test in the command fails against the base, so ONE genuinely discriminating test earned a PASS for every tautology sitting beside it in the same file, and named none of them. Hit for real: a three-test file where 2 of 3 were tautologies returned a bare PASS (Fine-Art-Archive audit finding F3). `_analyse_nodes` re-runs the candidate paths ALONE against the same extracted base with `pytest -v --tb=no` and reads the per-node outcomes: FAILED/ERROR `discriminates`, PASSED `hollow` (named by node id), SKIPPED/XFAIL/XPASS `inconclusive`. ONE extra subprocess, not one per node, sharing the caller's `timeout`. The result gains `hollow_nodes`, `node_verdict` and `node_analysis.counts`. ADVISORY BY CONSTRUCTION. `verdict`, `ok` and the CLI exit code keep their exact meaning, so `runtime_ac`, `synthesis_promotion` and `record_verdict` read what they always read. The finding travels on `reason`, which is what `record_verdict` writes to `outcomes.notes`, and node ids go into the completion event's `test_ids` -- the only test-identity field the payload schema has, since an extra key is REJECTED by `feedback._sanitize_completion_payload`. Scoping the probe to `test_paths` rather than the whole `test_cmd` tightens the evidence a second way: a failure in some unrelated test the command happens to run can no longer read as this change's proof. WHEN IT CANNOT ATTRIBUTE IT SAYS SO, with the prerequisite NAMED -- no pytest, a collection error, non-Python paths, a timeout -- and a PASS carrying an INDETERMINATE analysis appends that to its own reason. An empty hollow list and "could not look" read identically otherwise, which is the masking this change exists to remove. Two under-reporting traps closed, each proven by break->revert: - the probe parses UNTRUNCATED stdout. `_run`'s 4000-char tail drops the earliest nodes of any sizeable file. - the probe clears inherited `PYTEST_ADDOPTS`. `-x` stops at the first failing node -> false clean `node_verdict: PASS` with zero hollow nodes; `-n auto` activates xdist, whose verbose lines put the outcome BEFORE the node id -> nothing parses. Both measured with the guard removed. DEDUP (CLAUDE.md 0): hollow detection exists twice, both at coarser grain -- `local_verify.verify` per COMMAND and `objective_anchor.arm_signals` per ARM (base_pass boolean) -- and `testgen_gate.pytest_cmd(collect_only=True)` builds a pytest collection command but reads only `ok`. No per-node parser or attribution exists anywhere in the tree. The improvement log records this precision limit as STILL OPEN; extended `local_verify` rather than building anything new, and the `deliberate-break-verifier` capability's lifecycle state is unchanged. Selftest adds the mixed real+tautology fixture on the existing `_init_repo`, with a DELIBERATE BREAK -> REVERT on `NODE_OUTCOME_DISPOSITIONS`: count a base-passing node as part of the proof and the run reverts to exactly the old masking PASS. Also pins the two parse shapes that would under-report (an outcome word inside a summary error message; xdist's outcome-first line) and the named-prerequisite INDETERMINATE path. It needs only git and pytest, so it runs on a bare runner -- NO ceiling moved and nothing new is skipped. NO FLOOR CHANGE, and that is measured not assumed: the new coverage is in the module selftest, so pytest collects the same 442. Re-measured on the merge result after rebasing onto origin/main 146b458 (#98 and #91 landed underneath): 442 passed / 442 collected / floor 442, 85/85 selftests, 5/5 gates, mypy ratchet 20/20 unchanged and `local_verify` stays off the exempt list. Deliberately NOT recorded as a `capability_propensity.record_repair`: the `deliberate-break-verifier` repair proposal was drained on 2026-08-23 for the unrelated NO_BINDING-surface defect in PR #32, and this precision limit was never in that proposal's defect evidence. Recorded as its own finding on improvement-log item 0 instead. Co-authored-by: Tim Stranske <tim@stranskemo.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Batch three of the mypy campaign: 372 -> 338 findings, exempt bound 26 -> 20, 79 of 99 modules checked.
Two shadowed-name bugs the checker found
Same shape as the
consumer_sync_shadowone in batch 1, and it keeps recurring here:repo_knowledge.main()boundrepothree times — astrfor--export-agents-mdand two optionals for--repo. The optionals are renamedrepo_argalong with every use inside their blocks, so the two meanings stop sharing a name.no-redefwas the only thing pointing at it.capability_activation_audit's selftest reusedbrokenfor both a str and the note's return.Patterns, fixed once at the declaration
SEEDinrepo_knowledgeis a nested literal, so all five reads widened toobject. One annotation cleared them.summary,out,report_repos) whose literal pins a narrow value type, then conflicts on+= 1/.append(...).cast, notlist(...), for object-typed reads —list(x)doesn't satisfy mypy for anobjectand adds a real copy. Batch 2 taught this; two more sites had it.The
Mappingfix propagated, which is the right directionBatch 2 changed
tick'senvtoMapping[str, str], which pushed five errors downstream into callees still declaringdict.adversarial.review_enabled,adversarial.reviewers_from_envandroles' env parameters now accept aMappingtoo — they only ever read from it, sodictwas never what they needed.Two narrow ignores, each with its reason: a deliberately mixed-type loop probe in
durability_sweep, and onesetdefault(...)index where the row is genuinely heterogeneous. No error code is disabled anywhere.Test gate
python3 src/verify.py— 442 passed, 0 failed, 0 skipped, 85/85 selftests, 43/43 can-fire, 5/5 gates. ruff and black clean.🤖 Generated with Claude Code